That's cool feature 💯

hi @joeriddles what will be the official middleware repo for your package?

Thanks for the review @pkieltyka! I have renamed the file and variables you requested in this commit: 5cf9bc3

hi @joeriddles what will be the official middleware repo for your package?

Do you mean I should add a new middleware file, like middlewares/find.go? I can add something like this that allows the user to pass a function that should be called with the found route pattern.

@pkieltyka I have added a new middleware here: f6c1df8. I also updated the example I added to use the new FindPattern middleware. Thanks again for reviewing!

👀 🙌🏾

any news?

@pkieltyka any chance this gets merged soon?

I am going to fork the repo and merge this PR in the fork 👀

@VojtechVitek Can you make a new release please?

Thanks for merging @VojtechVitek!

@joeriddles Is it expected behaviour? It seems to truncate the actual path:

m := chi.NewMux()
m.Route("/yo", func(r chi.Router) {
	r.Get("/sup", func(w http.ResponseWriter, r *http.Request) {
		w.Write([]byte("sup"))
	})
})

fmt.Println(m.Find(chi.NewRouteContext(), http.MethodGet, "/yo/sup")) // "/sup", but expected "/yo/sup"

@joeriddles @dils2k

btw: I have just found out that we already have this rctx.RoutePattern() method on RouteContext:

func Instrument(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		next.ServeHTTP(w, r)
		routePattern := chi.RouteContext(r.Context()).RoutePattern()
		measure(w, r, routePattern)
	})
}

Isn't it what we were after?

@joeriddles Is it expected behaviour? It seems to truncate the actual path:

m := chi.NewMux()
m.Route("/yo", func(r chi.Router) {
	r.Get("/sup", func(w http.ResponseWriter, r *http.Request) {
		w.Write([]byte("sup"))
	})
})

fmt.Println(m.Find(chi.NewRouteContext(), http.MethodGet, "/yo/sup")) // "/sup", but expected "/yo/sup"

No, that is not expected behavior. I will create a fix.

@joeriddles @dils2k

btw: I have just found out that we already have this rctx.RoutePattern() method on RouteContext:

Isn't it what we were after?

The goal of Find is to determine the matched route pattern before the request is handled, similar to the existing Match function.

See the issue description:

I am working on adding a middleware for OpenTelemetry to a Chi-based service that needs to send a metric before the request has executed. We need to include a string for the API endpoint, like the route pattern, in the metric. This is currently not possible due to the way request route patterns are resolved using Chi's trie-based router, as noted by the maintainer in this issue #692 (comment).

@joeriddles @dils2k

btw: I have just found out that we already have this rctx.RoutePattern() method on RouteContext:

func Instrument(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		next.ServeHTTP(w, r)
		routePattern := chi.RouteContext(r.Context()).RoutePattern()
		measure(w, r, routePattern)
	})
}

Isn't it what we were after?

The issue with RoutePattern is that it returns the pattern only if the request successfully reaches the route see here.

However, in some cases (such as collecting metrics), it's necessary to determine the route pattern before the request is fully executed. This is important because requests can fail or be declined by authentication middleware.

Cool, makes sense. Thanks for the explanation 👍

@joeriddles I'll hold off with releasing new minor version until the fix is ready. Please keep me posted.

@joeriddles any updates?

This is blocking the release, so I'm tempted to revert this PR until a fix for #872 (comment) is ready. Thoughts?

This is blocking the release, so I'm tempted to revert this PR until a fix for #872 (comment) is ready. Thoughts?

@VojtechVitek a follow-up PR is created here: #954. Thanks!

@joeriddles any updates?

This is blocking the release, so I'm tempted to revert this PR until a fix for #872 (comment) is ready. Thoughts?

Do you have any updates on when we might expect the release?

@VojtechVitek